fix: Bump Bouncy Castle to fix CVE-2025-8916 - #1453
lukaszsocha2 wants to merge 8 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| token: ${{ secrets.DISPATCH_ACCESS_TOKEN }} | ||
|
|
||
| - name: Set up Git | ||
| run: | | ||
| git config --global user.name 'box-sdk-build' | ||
| git config --global user.email 'box-sdk-build@box.com' | ||
| - name: Fetch all branches and tags | ||
| run: git fetch --prune --unshallow | ||
|
|
||
| - name: Auto update pull requests | ||
| run: | | ||
| PR_LIST=$(curl -s -H "Authorization: Bearer ${{ secrets.DISPATCH_ACCESS_TOKEN }}" "https://api.github.com/repos/$GITHUB_REPOSITORY/pulls?state=open" | jq -r '.[] | .head.ref') | ||
| for pr_branch in $PR_LIST; do | ||
| git checkout "$pr_branch" | ||
| if git merge origin/sdk-gen; then | ||
| git push | ||
| else | ||
| # Conflict occurred, resolve by keeping our changes | ||
| git checkout --ours . | ||
| git add . | ||
| git commit -m "Auto resolve conflict by keeping our changes" | ||
| git push | ||
| fi | ||
| done |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 year ago
The best way to fix the problem is to add a permissions block to the workflow file. This should be added at the root level (alongside or beneath the name key) to ensure all jobs will inherit these minimal permissions. Based on the workflow steps, which include pushing to the repository and updating pull requests, the minimal required permissions are likely contents: write (to push branches) and pull-requests: write (to update PRs). If the workflow only requires reading contents, then contents: read would be sufficient, but here, git push is in use, so write is needed.
You should add this block as the second entry in the file:
permissions:
contents: write
pull-requests: writeNo changes to imports, methods, or other code are needed. Only the explicit permissions block is required in .github/workflows/autoupdate-pr.yml.
| @@ -1,4 +1,7 @@ | ||
| name: Autoupdate PR | ||
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
| on: | ||
| push: | ||
| branches: |
4bdfa5b to
df832fd
Compare
Pull Request Test Coverage Report for Build #4931Details
💛 - Coveralls |
lukaszsocha2
changed the title
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| - name: Setup Java | ||
| uses: actions/setup-java@v4 | ||
| with: | ||
| distribution: 'temurin' | ||
| java-version: '8' | ||
| - name: All Tests | ||
| if: startsWith(github.head_ref, 'codegen-release') | ||
| env: | ||
| JAVA_COLLABORATOR_ID: ${{ secrets.JAVA_COLLABORATOR_ID }} | ||
| JAVA_COLLABORATOR: ${{ secrets.JAVA_COLLABORATOR }} | ||
| JAVA_ENTERPRISE_ID: ${{ secrets.JAVA_ENTERPRISE_ID }} | ||
| JAVA_JWT_CONFIG: ${{ secrets.JAVA_JWT_CONFIG }} | ||
| JAVA_USER_ID: ${{ secrets.JAVA_USER_ID }} | ||
| run: ./gradlew integrationTest --stacktrace | ||
| - name: Smoke Tests | ||
| if: "!startsWith(github.head_ref, 'codegen-release')" | ||
| env: | ||
| JAVA_COLLABORATOR_ID: ${{ secrets.JAVA_COLLABORATOR_ID }} | ||
| JAVA_COLLABORATOR: ${{ secrets.JAVA_COLLABORATOR }} | ||
| JAVA_ENTERPRISE_ID: ${{ secrets.JAVA_ENTERPRISE_ID }} | ||
| JAVA_JWT_CONFIG: ${{ secrets.JAVA_JWT_CONFIG }} | ||
| JAVA_USER_ID: ${{ secrets.JAVA_USER_ID }} | ||
| run: ./gradlew smokeTest --stacktrace |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 year ago
To fix this problem, add a permissions block at the top level of the workflow YAML (i.e., before jobs:). This restricts the default permissions granted to GITHUB_TOKEN for all jobs in the workflow that do not have their own permissions section. Since the job does not require write access or special permissions, assign contents: read as the default, which allows jobs only read access to repository contents. No other modifications are needed; simply insert the block immediately after the workflow name: or before jobs:.
| @@ -1,4 +1,6 @@ | ||
| name: Integration tests sdk | ||
| permissions: | ||
| contents: read | ||
| on: | ||
| pull_request: | ||
| branches: |
lukaszsocha2
changed the title
CVE-2025-8916
| if (trustManager != null) { | ||
| try { | ||
| SSLContext sslContext = SSLContext.getInstance("SSL"); | ||
| sslContext.init(null, new TrustManager[] {trustManager}, new java.security.SecureRandom()); |
Check failure
Code scanning / CodeQL
`TrustManager` that accepts all certificates High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 year ago
General fix approach:
Replace usage of TrustAllTrustManager with either the default TrustManager (which performs standard validation), or, if supporting self-signed certificates, use a TrustManager created from a KeyStore containing only the specific trusted test certificates. For tests, avoid setting a TrustManager that accepts all certificates.
Specific fix steps:
- In
BoxAPIConnectionForTests.java, remove or replace all instances ofnew TrustAllTrustManager()with a TrustManager created from a test KeyStore or the default TrustManager. - If test APIs require trust of test self-signed certificates, initialize a
KeyStore, add the specific test certificate, and then useTrustManagerFactoryto construct the TrustManager. - For tests that do not require custom certificate handling, simply do not call
configureSslCertificatesValidation, so the SDK uses the default SSL context and TrustManager.
File/regional/line changes:
- In every constructor of
BoxAPIConnectionForTeststhat currently doesconfigureSslCertificatesValidation(new TrustAllTrustManager(), ...), either remove this call (if not necessary), or replace it with use of a TrustManager obtained from the system's default TrustManagerFactory (or from a KeyStore containing only the explicit test certificates). - Define a new helper method in
BoxAPIConnectionForTests.javato get the default TrustManager. - Remove all usage and imports of
TrustAllTrustManagerif possible.
Required methods/imports:
- Import
TrustManagerFactory,KeyStore, and relevant SSL classes if not present. - Optionally, define a utility method in the test class to load trusted certificates for test purposes only.
| @@ -4,37 +4,57 @@ | ||
| import static okhttp3.ConnectionSpec.MODERN_TLS; | ||
|
|
||
| import java.util.Arrays; | ||
| import java.security.KeyStore; | ||
| import javax.net.ssl.TrustManagerFactory; | ||
| import javax.net.ssl.X509TrustManager; | ||
| import okhttp3.OkHttpClient; | ||
|
|
||
| class BoxAPIConnectionForTests extends BoxAPIConnection { | ||
| BoxAPIConnectionForTests(String accessToken) { | ||
| super(accessToken); | ||
| configureSslCertificatesValidation(new TrustAllTrustManager(), new AcceptAllHostsVerifier()); | ||
| configureSslCertificatesValidation(getDefaultTrustManager(), new AcceptAllHostsVerifier()); | ||
| } | ||
|
|
||
| BoxAPIConnectionForTests( | ||
| String clientID, String clientSecret, String accessToken, String refreshToken) { | ||
| super(clientID, clientSecret, accessToken, refreshToken); | ||
| configureSslCertificatesValidation(new TrustAllTrustManager(), new AcceptAllHostsVerifier()); | ||
| configureSslCertificatesValidation(getDefaultTrustManager(), new AcceptAllHostsVerifier()); | ||
| } | ||
|
|
||
| BoxAPIConnectionForTests(String clientID, String clientSecret, String authCode) { | ||
| super(clientID, clientSecret, authCode); | ||
| configureSslCertificatesValidation(new TrustAllTrustManager(), new AcceptAllHostsVerifier()); | ||
| configureSslCertificatesValidation(getDefaultTrustManager(), new AcceptAllHostsVerifier()); | ||
| } | ||
|
|
||
| BoxAPIConnectionForTests(String clientID, String clientSecret) { | ||
| super(clientID, clientSecret); | ||
| configureSslCertificatesValidation(new TrustAllTrustManager(), new AcceptAllHostsVerifier()); | ||
| configureSslCertificatesValidation(getDefaultTrustManager(), new AcceptAllHostsVerifier()); | ||
| } | ||
|
|
||
| BoxAPIConnectionForTests(BoxConfig boxConfig) { | ||
| super(boxConfig); | ||
| configureSslCertificatesValidation(new TrustAllTrustManager(), new AcceptAllHostsVerifier()); | ||
| configureSslCertificatesValidation(getDefaultTrustManager(), new AcceptAllHostsVerifier()); | ||
| } | ||
|
|
||
| @Override | ||
| protected OkHttpClient.Builder modifyHttpClientBuilder(OkHttpClient.Builder httpClientBuilder) { | ||
| return httpClientBuilder.connectionSpecs(Arrays.asList(MODERN_TLS, CLEARTEXT)); | ||
| } | ||
| /** | ||
| * Returns the system default X509TrustManager. | ||
| */ | ||
| private static X509TrustManager getDefaultTrustManager() { | ||
| try { | ||
| TrustManagerFactory tmf = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); | ||
| tmf.init((KeyStore) null); | ||
| for (javax.net.ssl.TrustManager tm : tmf.getTrustManagers()) { | ||
| if (tm instanceof X509TrustManager) { | ||
| return (X509TrustManager) tm; | ||
| } | ||
| } | ||
| throw new IllegalStateException("No X509TrustManager found"); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException("Failed to initialize default TrustManager", e); | ||
| } | ||
| } | ||
| } |
5 participants
No description provided.